Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove get_number_sequence foreign calls #3613

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

TomAFrench
Copy link
Member

@TomAFrench TomAFrench commented Nov 28, 2023

Description

Problem*

Resolves #1911

Summary*

This PR removes the get_number_sequence and get_reverse_number_sequence foreign calls and replaces their usage in tests with a mocked foreign call.

I'm leaving this as non-breaking as we make no guarantees about these functions and anyone relying on a foreign call to create an array of increasing numbers... shouldn't.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@TomAFrench TomAFrench marked this pull request as ready for review November 28, 2023 16:47
@TomAFrench TomAFrench requested a review from vezenovm November 28, 2023 16:47
Copy link
Contributor

@vezenovm vezenovm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add resolves #1911 to the description before merging so it auto closes the issue

Copy link
Contributor

github-actions bot commented Nov 28, 2023

Changes to circuit sizes

Generated at commit: 74bd134a1e8a08f8740bcb6f8b8649379bd839af, compared to commit: d983a08c7dce8fcf7dece1e0d5aff920b26cbdbb

🧾 Summary (10% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
brillig_oracle +6 ❌ +600.00% 0 ➖ 0.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
brillig_oracle 7 (+6) +600.00% 5 (0) 0.00%

@TomAFrench
Copy link
Member Author

Done, thanks for the reminder.

@TomAFrench TomAFrench enabled auto-merge November 28, 2023 16:52
@TomAFrench TomAFrench added this pull request to the merge queue Nov 28, 2023
Merged via the queue into master with commit e72f5af Nov 28, 2023
33 checks passed
@TomAFrench TomAFrench deleted the tf/remove-sequence-foreign-calls branch November 28, 2023 17:19
TomAFrench added a commit that referenced this pull request Nov 30, 2023
* master: (216 commits)
  chore(docs): address visibility issues in docs (#3643)
  chore: type formatting (#3618)
  fix: Restrict fill_internal_slices pass to acir functions (#3634)
  chore(docs): docs for v0.19.4 (#3601)
  feat: aztec-packages (#3626)
  chore: Move tests to the correct root (#3633)
  feat: Implement integer printing (#3577)
  fix: corrected the formatting of error message parameters in index out of bounds error (#3630)
  chore: Update ACIR artifacts (#3619)
  chore(debugger): Run debugger REPL in thread (#3611)
  chore: remove deprecated method (#3625)
  feat: Implement raw string literals (#3556)
  fix: docker builds (#3620)
  feat: Copy on write optimization for brillig (#3522)
  chore: separate methods in `dc_crate` into their own modules (#3585)
  feat: add special case for boolean AND in acir-gen (#3615)
  chore: Update ACIR artifacts (#3614)
  chore: remove `get_number_sequence` foreign calls (#3613)
  feat: Add package version to Nargo.toml metadata (#3427)
  chore(docs): Links to Aztec docs from errors (#3423)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(nargo): Remove unnecessary Brillig oracles used for testing
3 participants